Problem/Motivation
In a tableselect entity, one can supply an array of default values, but with a normal array with index starting at 0, the first element will never get checked by default.
In core function form_process_tableselect in form.inc, I notice that effectively, the index of the row is copied in the row's default checkvalue, so if the index is 0, it will get a 0 value and uncheck the box instead of check it. Since I didn't find any documentation stating that the default_values array must not start with element 0, I assume that this is a bug in the construction of the tableselect layout.
Steps to reproduce
Example:
for( $j=0; $j < $max; $j++ ) {
$default_value[$j] = 1;
}
$form['candidates2'] = array
(
'#type' => 'tableselect',
'#header' => $header,
'#options' => $table,
'#multiple' => true,
'#default_value' =>$default_value,
);
This leaves the first box unchecked, but if you supply the array
for( $j=1; $j <= $max; $j++ ) {
$default_value[$j] = 1;
}
Then all boxes are checked.
Proposed resolution
TBA
Remaining tasks
See #16
User interface changes
API changes
Data model changes
Release notes snippet
Comments
Comment #1
JvE commentedThe tableselect code did not take into account the special condition described in form_type_checkbox_value()
I have attached my attempt to fix this.
One patch with a test only which should fail.
One patch with both test and fix which should pass.
Comment #2
JvE commentedTag fix.
Comment #5
mgiffordI haven't included any of the tests. Seems everything's changed around since June 7, 2012 when this patch was first written.
Comment #6
jhedstromThe tests from the original patch should be ported over so this fix doesn't regress.
Comment #7
mgiffordOk, that's essentially a re-roll with the tests rebuilt from the earlier patch.
I tagged it "needs reroll" hoping that someone takes it on.
Comment #8
jaipal commentedDone need reroll.
Comment #9
David Hernández commentedHello!
Thank you for working on this issue!
We should all try and use the same sprint tag. According to https://groups.drupal.org/node/447258 it should be SprintWeekend2015 with no #.
Comment #10
idebr commented@jaipal The patch you uploaded is a lot bigger than the previous patch. This is probably due to tabs vs. spaces in your editor configuration. Please have a look at the .editorconfig file to see if your IDE coding style matches the Drupal coding standards.
Comment #11
charginghawk commentedComment #12
charginghawk commentedFixed the last patch's indentation and 'else' formatting to match Drupal coding standards.
Comment #13
charginghawk commentedOops! Forgot to set status.
Comment #14
charginghawk commentedComment #15
manningpete commentedComment #16
jhedstromThe patch in #12 lost the tests from #1 again.
Comment #29
quietone commentedTriaged in bugsmash with myself and Lendude. We agree that this is still valid. Lendude found that rendering needs to be checked to determine if the check is actually checked.
The patch needs to be updated so adding the tag. To anyone doing that work read the remaining tasks in the issue summary.
Comment #30
prem suthar commentedRe-roll the Patch For 9.5 version.
Comment #31
bhanu951 commentedComment #32
smustgrave commentedThis issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.
As a bug this will need a test case